Skip to content

Filter occurrences by the algorithms that actually ran, and make class masking safe to re-run#1368

Merged
mihow merged 16 commits into
mainfrom
fix/algo-filter-and-masking-idempotency
Jul 23, 2026
Merged

Filter occurrences by the algorithms that actually ran, and make class masking safe to re-run#1368
mihow merged 16 commits into
mainfrom
fix/algo-filter-and-masking-idempotency

Conversation

@mihow

@mihow mihow commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #999 (class masking). Two rough edges surfaced while running class masking on a project in a production deployment.

First, the occurrence list could not be filtered down to what a given algorithm actually produced. The filter's choices came from pipeline configuration, so a standalone post-processing algorithm such as a class-masked classifier — which has no pipeline — never appeared, and neither did a superseded model version whose determinations still exist. The filter itself also had a counting bug that inflated result counts, and filtering by a detector returned nothing at all.

Second, class masking was only safe to run once per capture set: if a source classification became terminal again after a later dedup or re-classification pass, the source could be re-scored and pick up a duplicate masked classification.

This PR addresses both. The occurrence algorithm filter now offers exactly the algorithms that produced results in the project — every classifier, detector, and post-processing algorithm whose output still exists — served from a new endpoint, /occurrences/algorithms/. The project's algorithms page is unchanged from main: it keeps showing the algorithms on the project's enabled pipelines, so a freshly configured project still sees what it can run before any job has completed. Class masking is now idempotent: a source is masked at most once per masking algorithm.

No schema change and no migration. Every fix is query-level.

List of Changes

Change (user-facing effect) How (implementation)
The occurrence algorithm filter offers exactly the algorithms that produced results in the project — including older/superseded versions, detectors, and standalone post-processing algorithms — so it never lists a choice with zero results. New OccurrenceViewSet sub-action at /api/v2/occurrences/algorithms/, backed by a reusable queryset method Algorithm.objects.used_in_project(): the algorithm ids owning classifications (classifiers, post-processing) or detections (detectors) whose capture belongs to the project. Requires project_id and applies the project visibility check.
Filtering occurrences by an algorithm now matches detectors too, and the result count is no longer inflated when an occurrence has several matching classifications. New OccurrenceQuerySet.processed_by_algorithm() / not_processed_by_algorithm(), backed by an EXISTS subquery matching a detection's detection_algorithm or a classification's algorithm. OccurrenceAlgorithmFilter routes through them. The name covers post-processing algorithms (a size filter neither detects nor classifies, but its classifications match here); processed_by_pipeline and processed_by_job can follow the same family later.
The project's algorithms page behaves as on main: it lists the algorithms on the project's enabled pipelines, so a new project sees its available algorithms before anything has run. AlgorithmViewSet.get_queryset keeps main's enabled-pipeline filter; this PR only adds a comment pointing to the new choices endpoint.
Clicking an algorithm name on an occurrence's classification still opens its detail page (and category map) even when that algorithm is not on an enabled pipeline. The detail endpoint is unscoped by design; this PR pins that with a regression test.
Re-running class masking (to finish an interrupted run, or after a dedup / re-classification pass) no longer creates duplicate masked classifications. _scoped_classifications excludes sources that already have a masked child for the same masking algorithm, keyed on the applied_to lineage rather than the mutable terminal flag.

Two endpoints, two purposes

The project algorithm list and the occurrence filter choices sound alike but answer different questions, so they are served separately:

  • /ml/algorithms/?project_id=… (the algorithms page) answers "what can this project run?" — the algorithms on its enabled pipelines. Configuration-driven, non-empty for a freshly configured project.
  • /occurrences/algorithms/?project_id=… (the filter choices) answers "what has produced results here?" — anything whose output still exists, regardless of pipeline configuration. Results-driven, so filtering by any choice returns at least one occurrence.

An earlier revision of this PR merged the two into one used-only list with historical entries grayed out. That left a new project's algorithms page empty until its first run, which is the wrong page to be empty — the user is there to decide what to run. Splitting the consumers resolved it and removed the extra enabled_in_project flag the merged design needed.

A follow-up (planned alongside a denormalised project column on classifications/detections) can make the algorithms page all-inclusive — configured ∪ used, with per-algorithm usage counts and links into filtered occurrences.

The occurrence filter counting bug

OccurrenceAlgorithmFilter previously filtered with detections__classifications__algorithm__in=…. That is a multi-valued join, so an occurrence with three matching classifications became three rows, and with no .distinct() downstream the paginator's COUNT counted it three times. On the largest project, filtering by one classifier reported 111,272 occurrences where there are 55,214. The same join matched only classifiers — a detector authors no classification, so filtering by a detector returned nothing at all. The EXISTS form counts each occurrence once and matches both roles.

What was measured

All figures are from a local copy of production (~834k classifications platform-wide), on the largest project unless noted. They are indicative — a working database, not an isolated benchmark rig — so read them as order-of-magnitude, not precise. Timing loops ran with cachalot_disabled() and rebuilt the queryset each iteration (a reused queryset serves its result cache and reports a false ~0 ms).

Filter-choices query shapes (median wall-clock for the project-scoped used-only list):

Shape Median Why it was kept or rejected
Pipeline-only join (the algorithms page, unchanged) ~8 ms Index scan, but excludes superseded versions and post-processing algorithms — not "used".
Hybrid: pipeline join + restricted post-processing lookup ~12 ms An earlier revision here; fast, but still cannot express "used" (drops disabled-pipeline versions and pipeline-less detectors).
True used-only (used_in_project, this PR) ~0.1–0.6 s cold Correct definition. Scans classification and detection because neither has a project column; cachalot-warm after the first load. Served from /occurrences/algorithms/, which the UI requests when the filter panel opens — not on page load.
Authorship-only, no candidate restriction ~595 ms An earlier wrong revision; also silently dropped every detector.
Per-algorithm EXISTS loop ~941–1988 ms Rejected.
Pipeline OR used, one query ~2206 ms Rejected — COUNT(DISTINCT) over the multi-valued join dominates.

EXPLAIN (ANALYZE) findings:

  • The two-step form (materialize candidate ids, then algorithm_id IN (...)) plans as an index scan (~6 ms). Written as one join, the pipelines__isnull=True anti-join hides the ids at plan time and forces a parallel sequential scan of the whole classification table (~48 ms), whose cost grows with the platform's total classification count. This is why used_in_project materializes and sorts its id lists.
  • Neither Classification nor Detection has a project column — project is reachable only through source_image — so no index reaches the project directly. That is the structural reason the honest "used" query scans, and the reason its cost tracks total table size rather than a project's own volume.

Occurrence filter correctness (largest project, one classifier id):

Old join (detections__classifications__algorithm__in) New EXISTS (processed_by_algorithm)
COUNT reported 111,272 (inflated) 55,214 (correct)
Filter by a detector id 0 results matches the detector's occurrences

The measurement discipline behind all of this is now recorded in AGENTS.md: run EXPLAIN (ANALYZE) before claiming a plan, and measure on the largest project rather than a small one. An earlier revision of this PR claimed a plan it had not EXPLAINed and a speedup measured against the wrong baseline; both were corrected, and the rule is written down so the next change does not repeat it.

Tests

All green locally (CI-compose against the prod-copy database) and on the pushed head in GitHub CI.

  • TestAlgorithmViewSetProjectFilter (5) — the project algorithms page lists exactly the enabled pipelines' algorithms, including one that has never run; a disabled pipeline's algorithm is not listed; other projects are isolated; the unscoped list is unchanged; the detail endpoint resolves an algorithm outside the project's enabled set (the occurrence → algorithm name → category map link).
  • TestOccurrenceAlgorithmChoices (8) — /occurrences/algorithms/ returns exactly the algorithms with results in the project; a configured-but-never-run algorithm is not a choice; a superseded version whose determinations survive is; a detector that only ever produced detections is; classifications in another project do not leak; the underlying lookup deduplicates in SQL; project_id is required (400 without it); the response is the standard paginated {count, results} shape the frontend's entity picker consumes.
  • TestOccurrenceAlgorithmFilterQuerySet (5) — filtering by a classifier and by a detector each match the right occurrences; an occurrence with several matching classifications counts once (the counting bug above); exclusion is the strict complement of inclusion.
  • Class-masking suite (14, including the scope-shape tests merged from Stop large class masking jobs from being cancelled before they report any progress #1376) — including test_rerun_does_not_duplicate_masked_classifications: running the same mask twice, including the case where the source became terminal again between runs, yields exactly one masked classification. The Stop large class masking jobs from being cancelled before they report any progress #1376 scope-shape tests were updated for the idempotency guard's extra argument during the rebase.
  • Frontend: tsc --noEmit, ESLint, and Prettier pass on the changed files.

Notes / follow-ups

  • Denormalised project column on Classification / Detection is the durable fix for the cold cost of the used-only query, and the prerequisite for the all-inclusive algorithms page (configured ∪ used, with usage counts). Until then the choices endpoint pays a table scan on cache invalidation.
  • An algorithm that produced neither classifications nor detections in the project is not a filter choice — correct under "used".
  • The masking scope still filters on scores__isnull=False. As the scores field is retired in favour of logits-only masking, that filter should be revisited so logits-only classifications are not skipped.
  • The idempotency guard is keyed on (source classification, masking algorithm), and the masking algorithm's identity includes the taxa list by primary key rather than by contents. Editing a taxa list in place and re-running the same mask will skip already-masked sources and leave them scored against the old list; create a new taxa list to re-mask.
  • Running a different taxa list over an already-masked collection applies only to the sources the first mask left terminal, and reports that smaller count as if it were the whole collection. This predates the PR but sits close to the re-run safety it claims.

Closes the filterability and re-run gaps identified after #999.

Copilot AI review requested due to automatic review settings July 9, 2026 19:07
@netlify

netlify Bot commented Jul 9, 2026

Copy link
Copy Markdown

Deploy Preview for antenna-preview canceled.

Name Link
🔨 Latest commit 0306324
🔍 Latest deploy log https://app.netlify.com/projects/antenna-preview/deploys/6a618bc3fafd0f0008248bb6

@netlify

netlify Bot commented Jul 9, 2026

Copy link
Copy Markdown

Deploy Preview for antenna-ssec canceled.

Name Link
🔨 Latest commit 0306324
🔍 Latest deploy log https://app.netlify.com/projects/antenna-ssec/deploys/6a618bc34defb0000838d7ec

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@mihow, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5c32e434-693b-43ad-bf65-8c4da8c89b66

📥 Commits

Reviewing files that changed from the base of the PR and between 996277d and 0306324.

📒 Files selected for processing (11)
  • .agents/AGENTS.md
  • ami/main/api/views.py
  • ami/main/models.py
  • ami/main/tests.py
  • ami/ml/models/algorithm.py
  • ami/ml/post_processing/class_masking.py
  • ami/ml/post_processing/tests/test_class_masking.py
  • ami/ml/tests.py
  • ami/ml/views.py
  • ui/src/components/filtering/filters/algorithm-filter.tsx
  • ui/src/data-services/constants.ts
📝 Walkthrough

Walkthrough

This PR makes class masking reruns idempotent, changes project-scoped algorithm listing to use produced results, and adds occurrence filters that match detection or classification provenance by algorithm. Tests and query-performance guidance are updated.

Changes

Class Masking Idempotency

Layer / File(s) Summary
Idempotent masking scope
ami/ml/post_processing/class_masking.py
_scoped_classifications excludes sources with existing derived classifications from the masking algorithm, and run() passes that algorithm through.
Masking rerun validation
ami/ml/post_processing/tests/test_class_masking.py
Runs masking twice and verifies that no duplicate derived classification is created.

Algorithm Project Scoping

Layer / File(s) Summary
Project result lookup and listing
ami/ml/models/algorithm.py, ami/ml/views.py
Project-scoped algorithm lists use distinct classification and detection results produced in the active project.
Algorithm listing coverage
ami/ml/tests.py
Tests cover used, unused, superseded, detector, post-processing, orphan, unscoped, deduplicated, and cross-project algorithms.

Occurrence Algorithm Filtering

Layer / File(s) Summary
Machine-result queryset predicates
ami/main/models.py
Adds EXISTS-based include and exclude helpers for detection or classification results by algorithm.
Occurrence filter integration and tests
ami/main/api/views.py, ami/main/tests.py
Updates filter behavior and documentation, with coverage for matching, exclusion, and count deduplication.

Query Measurement Guidance

Layer / File(s) Summary
Query performance verification
.agents/AGENTS.md
Adds guidance for EXPLAIN ANALYZE validation and dataset-based timing measurements.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested labels: post-processing

Suggested reviewers: mohamedelabbas1996

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the two main changes: algorithm filtering and class-masking idempotency.
Description check ✅ Passed The description is thorough and matches the template with summary, changes, detailed description, testing, deployment notes, and impact notes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/algo-filter-and-masking-idempotency

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves operator usability and safety around class masking by (1) making post-processing algorithms selectable in a project’s algorithm filter whenever they have produced classifications in that project, and (2) making class masking idempotent so re-runs don’t create duplicate masked classifications.

Changes:

  • Expand AlgorithmViewSet.get_queryset list scoping to include algorithms that have classifications within the active project (not only those attached to enabled pipelines).
  • Make class masking re-runnable by excluding source classifications that already have a derived (masked) child for the same masking algorithm.
  • Add/extend tests to cover the expanded algorithm list and the masking idempotency behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
ami/ml/views.py Updates algorithm list scoping to union enabled-pipeline algorithms with algorithms that produced classifications in the project.
ami/ml/tests.py Adds API tests ensuring post-processing algorithms with in-project classifications appear, and cross-project leakage does not occur.
ami/ml/post_processing/class_masking.py Makes masking idempotent by excluding already-masked sources via applied_to lineage (reverse relation).
ami/ml/post_processing/tests/test_class_masking.py Adds regression test ensuring re-running the same mask does not duplicate masked classifications.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ami/ml/views.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ami/ml/views.py`:
- Around line 69-71: The `classified_in_project` query in `views.py` is too
broad and includes non-terminal classification rows, which can make disabled or
reprocessed pipeline algorithms appear in the list. Update the
`Classification.objects.filter(...)` call to scope the subquery to terminal
determinations only, using the existing
`Classification`/`detection__source_image__project` query path as the anchor.
Keep the same projection on `"algorithm"`, but add the appropriate
terminal-status filter so only final classifications are returned.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f21f62f6-12a4-4224-9ef9-fe8fbf58c16e

📥 Commits

Reviewing files that changed from the base of the PR and between 3f85d84 and 7a7b69b.

📒 Files selected for processing (4)
  • ami/ml/post_processing/class_masking.py
  • ami/ml/post_processing/tests/test_class_masking.py
  • ami/ml/tests.py
  • ami/ml/views.py

Comment thread ami/ml/views.py Outdated
Comment thread ami/ml/views.py Outdated
@mihow

mihow commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Claude says: Correcting something I got wrong earlier in this thread, and reporting a bug a structural review pass caught in the change I had pushed. Both are now fixed in 0a6dfa0f, but the record needs setting straight.

The performance claim I posted was measured against the wrong baseline

I said the change made the list "about four times faster". That compared against an intermediate commit on this branch, not against main. Main was never the pipeline-OR-used shape — it is the pipeline join alone, which touches only small, rarely-written tables. Measured properly, against a local copy of production data on the largest project:

shape median
main today (pipeline join) 8 ms
authorship-only (what I had pushed) 595 ms

So that commit was roughly a 74x regression, not a speedup. Compounding it, the new query depends on main_classification, which cachalot invalidates on every classification insert — continuously during processing, which is exactly when people are looking at these lists. I apologise for the misleading number; it came from benchmarking two variants of my own change against each other and never re-baselining against main.

It also silently dropped every detector

Scoping the list by classification authorship looked equivalent to "algorithms used in this project", but it is not. Localizers set Detection.detection_algorithm and never write a Classification row (ami/ml/models/pipeline.py), so the detector behind every detection in a project vanished from the list. On the largest project that removed a FasterRCNN localizer that had genuinely run. The /projects/<id>/algorithms page renders a task_type column specifically to show detectors alongside classifiers, so this was visible data loss, not a cosmetic change.

What it does now

The enabled-pipeline join is the primary rule again, and post-processing algorithms are admitted by a second query restricted to algorithms with no pipeline — a handful of candidates, which is what keeps the classification lookup off the full table. The two are collected separately rather than OR-ed, because an OR across the pipeline join forces the SELECT DISTINCT that made the earlier variant slow.

shape median detectors post-processing algorithms
main today 8 ms yes no — the bug this PR fixes
authorship-only (reverted) 595 ms no yes
now 46 ms yes yes

On the largest project the result is a strict superset of main: nothing main listed is lost, and the four post-processing algorithms are added.

The one thing this gives up, which is your call

It no longer hides algorithms that are configured for the project but never ran — the thing you asked for further up this thread. I went with the pipeline join anyway because your ask was conditional ("it needs to be fast to load"), and hiding them is what costs the 8 ms to 595 ms and the detectors. Hiding unused algorithms cheaply needs a denormalised project on Classification, the way Occurrence already has one; that is a larger change than this PR. If you would rather have the filtering and accept the cost, it is a one-line revert of 0a6dfa0f — but I would not recommend it in this form.

Also still absent, exactly as on main: an algorithm attached to no pipeline that only ever produced detections. Catching it means scanning the detection table.

@mihow

mihow commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Claude says: A structural review pass found two more things in the algorithm-list query. One is fixed in 7d67f5be; the other is a known gap I am documenting rather than closing here.

Fixed: the post-processing lookup shipped one row per classification

The join from an algorithm to a project through its classifications emits one row per matching classification, not one per algorithm. So the second query was fetching every masked classification id in the project — 846 rows for 4 algorithms on one project, 1354 rows for a single algorithm on another — only to collapse them into a handful of ids in Python. Harmless at todays volumes (still tens of milliseconds), but the row count was linear in masked-classification count, which is exactly what a production masking run generates. 7d67f5be adds .distinct() so the database returns one id per algorithm. A test now pins the row count of the underlying lookup, since the listed names are correct either way and the bug is invisible in the output.

For the record, the reviewer suggested Exists() for this rather than .distinct(). I measured both, and Exists() is the wrong call here: it ran 223 ms on one project and 1239 ms on another, versus roughly 37 ms and 49 ms for .distinct(). Exists() short-circuits when proving an algorithm has a classification, but the query has to run the subquery to completion for every algorithm that does not, and most unpipelined algorithms have none in a given project. .distinct() keeps the single scan and just deduplicates it.

Known gap: an algorithm on a pipeline not enabled here, but with determinations here

The second query is restricted to algorithms with no pipeline at all, so an algorithm attached to a pipeline that exists but is not enabled for this project stays out of the list even when it owns classifications here. This is real in the data: on one project, Turing Kenya and Uganda Species Classifier has 207 classifications but sits on a pipeline not enabled there, so its determinations are unfilterable. That is the same class of gap this PR set out to fix, now closed for standalone post-processing algorithms but not for this shape.

I left it out on purpose. Closing it means widening the candidate set from "no pipeline" to "not configured-and-enabled here", which drops the pipelines__isnull=True guard and puts the classification join back over a larger candidate set — measured at roughly five times the runtime (about 175-275 ms versus 40-50 ms). It is a reasonable follow-up if operators hit it, but it is a separate scope decision with its own cost, not a fix that belongs quietly in this PR. Flagging for your call.

@mihow

mihow commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Claude says: One more correction, and a genuine speedup that came out of it. A review pass pointed out that a mechanism I described three times (PR body, an earlier comment, the code comment) was not what the database actually did — and checking it with EXPLAIN showed they were right.

I had said that restricting the post-processing lookup to unpipelined algorithms "keeps it off the full classification table". It did not. Written as a single join —

Algorithm.objects.filter(pipelines__isnull=True, classifications__detection__source_image__project=project)

— the pipelines__isnull=True anti-join hides the candidate algorithm ids at plan time, so Postgres cannot use the algorithm_id index and sequentially scans the whole main_classification table (a parallel seq scan of ~278k rows per worker, ~48 ms). Its cost tracks the platform-wide classification count, and it is scanned cold on every cachalot invalidation, i.e. on every classification write.

Fixed properly in e691c035 by doing it in two steps: collect the unpipelined algorithm ids first as an explicit list, then filter Classification.objects.filter(algorithm_id__in=ids, detection__source_image__project=project). With a literal IN list the planner uses the index. EXPLAIN confirms the plan flips from a seq scan to an index scan, ~48 ms to ~6 ms, and the whole endpoint drops from ~46 ms to ~12 ms. It is now flat in table size rather than linear, which was the property I had wrongly claimed the first version already had.

One footgun handled in the same commit: Classification has a default Meta.ordering, and a .values_list(...).distinct() silently folds the ordering columns into the DISTINCT, which would return one row per classification again. .order_by() before .distinct() clears it; the dedup test now pins that.

So the honest mechanism is: the enabled-pipeline join covers detectors and configured classifiers; the two-step lookup adds standalone post-processing algorithms via their classifications, indexed and flat in table size. Apologies for the earlier hand-wave — I asserted the plan I wanted rather than the plan Postgres produced, which is exactly the thing an EXPLAIN would have caught, and did once someone looked.

Comment thread ami/ml/views.py
# Classification and so cannot be found by their results at all.
configured_for_project = Algorithm.objects.filter(
pipelines__project_pipeline_configs__project=project,
pipelines__project_pipeline_configs__enabled=True,

@mihow mihow Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am okay with this compromise if it's the only way to make the query fast until we add a denormalized project column to the Classification table. BUT it is not accurate. The idea is to see algorithms actually used even if they are no longer enabled (e.g. old versions). Also, Detection algorithms can be retrieved from the classification__detection relationship.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude says: Went with the accurate definition rather than the compromise. used_in_project now includes old/superseded versions whose determinations still exist (pipeline enablement is not consulted) and reaches detectors through Detection.detection_algorithm, exactly as you described.

The honest cost: neither Classification nor Detection has a project column, so the lookup scans — a project is reachable only through source_image. Measured ~0.1–0.6 s cold on the larger projects against the prod copy, cached warm by cachalot. The fast hybrid (~12 ms) is a one-method revert if that cold cost is a problem before the denormalized project column lands, but it can't express "used" without dropping the old versions and the pipeline-less detectors, so the accurate one is what's in. Flagged the denormalized column as the durable fix in the PR notes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude says: Final shape, per your note about graying: one list serves both consumers instead of splitting semantics. used_in_project returns everything that ran (old versions + detectors + post-processing), and each row now carries enabled_in_project — true if it's on a pipeline the project has enabled, false if it only ran historically. The occurrence filter uses the full set; the algorithms page shows the full set but grays the false rows (new optional rowClassName on the shared Table). So old/superseded versions stay filterable and are visibly marked as no-longer-enabled rather than hidden.

Honest cost is unchanged (~0.1–0.6 s cold, cachalot-warm after) since it's the same used_in_project query; the enabled_in_project flag is a cheap per-row Exists on top. Full measurement table (algorithm-list shape ladder, EXPLAIN index-vs-seqscan, and the occurrence-filter 111,272-vs-55,214 count fix) is in the updated PR description. Pushed in 68dc34b3.

Comment thread ami/ml/views.py Outdated
@mihow mihow changed the title Show post-processing algorithms in project filters and make class masking safe to re-run Filter a project by the algorithms that actually ran, and make class masking safe to re-run Jul 21, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/AGENTS.md:
- Around line 22-23: Revise the index-usage statement in the guidance around the
field__in literal-id example to say it can enable an index-based plan rather
than guaranteeing one. Retain the requirement to verify the actual plan with
EXPLAIN (ANALYZE), and avoid claiming that PostgreSQL will always use the index.

In `@ami/main/tests.py`:
- Around line 6566-6570: Update the Detection fixture in the test creating
detection to use integer pixel coordinates in bbox, changing the current float
values to [0, 0, 1, 1] while leaving the other fields unchanged.

In `@ami/ml/models/algorithm.py`:
- Around line 160-198: Update AlgorithmQuerySet.used_in_project in
ami/ml/models/algorithm.py:160-198 to start with algorithms from enabled project
pipelines, then union only standalone post-processing algorithms found through
classifications; remove the unrestricted Detection.detection_algorithm lookup.
Update the contract in ami/ml/tests.py:2040-2048, fixtures in
ami/ml/tests.py:2058-2081 so enabled-but-unused algorithms remain included while
disabled-pipeline output is excluded, and reverse the result-only assertions in
ami/ml/tests.py:2103-2136 to verify detector inclusion through its enabled
pipeline rather than standalone detection authorship.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: af126113-c85e-4c03-9f19-39679eb746b5

📥 Commits

Reviewing files that changed from the base of the PR and between 87a68ad and 996277d.

📒 Files selected for processing (8)
  • .agents/AGENTS.md
  • ami/main/api/views.py
  • ami/main/models.py
  • ami/main/tests.py
  • ami/ml/models/algorithm.py
  • ami/ml/post_processing/class_masking.py
  • ami/ml/tests.py
  • ami/ml/views.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • ami/ml/views.py
  • ami/ml/post_processing/class_masking.py

Comment thread .agents/AGENTS.md
Comment thread ami/main/tests.py
Comment thread ami/ml/models/algorithm.py
@mihow mihow changed the title Filter a project by the algorithms that actually ran, and make class masking safe to re-run Filter occurrences by the algorithms that actually ran, and make class masking safe to re-run Jul 22, 2026
mihow and others added 13 commits July 22, 2026 16:46
…s algorithm list

The project-scoped algorithm list only returned algorithms attached to an
enabled pipeline config. Post-processing algorithms such as class masking are
created standalone with no pipeline, so they were hidden from the project's
algorithm filter even though they produce determinations in the project. The
list now also includes any algorithm that produced classifications in the
project, so an operator can filter occurrences by a masked result. The detail
endpoint was already unscoped; this brings the list in line with it.

Co-Authored-By: Claude <noreply@anthropic.com>
Class masking previously relied only on the terminal flag it flips on the
source classification to avoid re-scoring it again. If a source became terminal
again (after a dedup or re-classification pass), or a partially completed run
was retried, the source could be masked a second time and gain a duplicate
masked classification. The scope now also excludes sources that already have a
masked child for the same masking algorithm, keyed on the applied_to lineage,
so a source is masked at most once per masking algorithm. This makes finishing
an interrupted run safe.

Co-Authored-By: Claude <noreply@anthropic.com>
… ran

List an algorithm for a project when it produced classifications there, and drop
the enabled-pipeline branch entirely. A configured but never-run algorithm has no
results to filter or inspect, so listing it only adds dead entries to the filter.

Removing the branch also removes the OR across the pipeline join and the
SELECT DISTINCT it forced. Measured against a local copy of production data
(roughly 834k classifications), on one of the larger projects the paginated
endpoint drops from 2206 ms to 556 ms, most of it in the pagination COUNT:
1722 ms to 275 ms.

Both remaining shapes still sequentially scan the classification and detection
tables, because no index reaches a project from a classification. The cost
therefore tracks total table size rather than project size, and improving it
further needs a denormalised project on Classification or a precomputed
per-project list.

Co-Authored-By: Claude <noreply@anthropic.com>
…lly covers

The docstring credited the lineage guard with resuming interrupted runs. It does
not: demoting a source and writing its masked child share one transaction, so a
resumed run already skips completed sources on the terminal flag alone. The guard
covers only the case the flag cannot, a source restored to terminal by a later
dedup or re-classification pass.

Also record that a taxa list is identified by primary key rather than by contents,
so editing a list in place and re-running the same mask leaves already-masked
sources scored against the earlier membership.

Co-Authored-By: Claude <noreply@anthropic.com>
The subquery returned one row per classification, so a project with a million
classifications fed a million rows into the IN clause to identify at most a few
dozen algorithms. Selecting distinct algorithm ids says what the query means.

Measured effect is small and close to run-to-run noise (median 610 ms -> 567 ms
over four runs against a local copy of production data); the change is for clarity.

Co-Authored-By: Claude <noreply@anthropic.com>
Scoping the list purely by classification authorship dropped every detector.
Localizers set Detection.detection_algorithm and never write a Classification,
so the detector behind every detection in a project disappeared from the
project's algorithm list, which renders a task_type column precisely to show
detectors next to classifiers.

Restore the enabled-pipeline join as the primary rule and admit post-processing
algorithms through a second query, restricted to algorithms with no pipeline so
the classification lookup stays off the full table. Collect the two separately
rather than OR-ing them: an OR across the pipeline join forces a SELECT DISTINCT
whose COUNT costs more than both queries together.

Measured against a local copy of production data, on the largest project the
list returns in 46 ms, versus 8 ms for the pipeline-only rule on main and 595 ms
for the authorship-only rule this replaces. The result is a superset of main:
nothing main listed is lost, and the post-processing algorithms are added.

An algorithm attached to no pipeline that only ever produced detections is still
absent, as it is on main. Catching it would mean scanning the detection table.

Co-Authored-By: Claude <noreply@anthropic.com>
The ids came from a set, whose iteration order is not a documented guarantee.
cachalot keys its cache on the generated query string, so a varying order of the
same ids would produce cache misses for identical results.

Also replaces the bound claimed for materialising the ids. "Dozens platform-wide"
was true when written but class masking creates an algorithm per source algorithm,
taxa list and reweight mode, so the count grows with use; the comment now says
where the growth comes from and when to switch to a subquery.

Co-Authored-By: Claude <noreply@anthropic.com>
The join from an algorithm to a project through its classifications emits one row
per matching classification, so the lookup shipped one id per masked classification
in the project — hundreds of thousands after a real masking run — to identify a
handful of algorithms. Adding DISTINCT collapses that to one id per algorithm in
the database. Measured effect is small on current data (the projects checked hold
hundreds to low thousands of masked classifications), but the row count was linear
in classification volume, which is exactly what this feature produces at scale.

Also document that an algorithm on a pipeline not enabled for the project stays
absent even when it has determinations there. Widening the second query to cover
that case measured roughly five times the runtime, so it is left as a known gap
matching the pipeline-join behaviour rather than folded into this change.

Co-Authored-By: Claude <noreply@anthropic.com>
…N list

The single-join form (pipelines__isnull=True combined with the classification
relation) hides the candidate algorithm ids behind an anti-join at plan time, so
Postgres sequentially scans the whole classification table to answer it. Its cost
therefore grows with the platform's total classification count, and it is scanned
cold on every cachalot invalidation, which happens on every classification write.

Split it into two steps. Collect the unpipelined algorithm ids first, as an
explicit list, then filter classifications on algorithm_id IN (...). The planner
answers that from the algorithm_id index and touches only the rows for those
algorithms. EXPLAIN on a local copy of production data confirms the plan change:
a parallel sequential scan of main_classification (about 48 ms) becomes an index
scan (about 6 ms), and the whole endpoint drops from roughly 46 ms to 12 ms. The
first query is also cheap to cache, changing only when algorithms or pipeline
links change rather than on every classification.

Clear Classification's default ordering with .order_by() before .distinct(),
otherwise the ordering columns widen the DISTINCT back to one row per
classification. The dedup test now covers this.

Co-Authored-By: Claude <noreply@anthropic.com>
The unpipelined algorithm ids are rendered verbatim into the classification
lookup's IN clause. Algorithm orders by (name, version), so the ids came out in
name order, and identical membership could produce a different id order — and so
a different SQL string and cachalot cache key — across requests. Sorting makes the
inner query cache-stable, matching the outer id list, which was already sorted for
the same reason.

Co-Authored-By: Claude <noreply@anthropic.com>
Replace the pipeline-config + unpipelined-classification hybrid in
AlgorithmViewSet with a single reusable queryset method,
Algorithm.objects.used_in_project(). The list now contains exactly the
algorithms that produced results in the project — detectors via their
detections, classifiers and post-processing algorithms via their
classifications — regardless of whether the owning pipeline is still
enabled. A superseded model version stays listed as long as its
determinations survive; an algorithm configured on an enabled pipeline
that never ran no longer appears.

Add matching OccurrenceQuerySet methods, detected_or_classified_by() and
not_detected_or_classified_by(), and route OccurrenceAlgorithmFilter
through them. The filter previously joined detections__classifications,
which returned one row per matching classification and inflated the
paginator's COUNT, and it matched only classifiers — a detector produced
no match at all. The EXISTS form counts each occurrence once and matches
both roles, so every algorithm the list shows can now filter occurrences.

Document in AGENTS.md the two query-work rules this change was built on:
run EXPLAIN (ANALYZE) before claiming a query's plan, and measure on the
largest project (surveying several sizes) rather than a single small one.

Cost note: used_in_project scans classification and detection because
neither table has a project column, so it runs ~0.1-0.6s cold and scales
with total table size until a denormalized project column exists — the
honest cost of showing exactly what ran.

Co-Authored-By: Claude <noreply@anthropic.com>
…ray the rest

The project algorithm list returns every algorithm that produced results in
the project, including superseded versions and standalone post-processing
algorithms. Serve both consumers from that one list rather than splitting
semantics: the occurrence filter needs the full historical set, and the
algorithms page now shows the same set but grays out the algorithms that are
no longer enabled for the project.

Each algorithm in the project-scoped list carries `enabled_in_project` — true
when it is on a pipeline the project has enabled, false when it only ran
historically, and null on the unscoped list where there is no project for it
to be relative to. The frontend Algorithms table dims the false rows through a
new optional `rowClassName` on the shared Table component.

Co-Authored-By: Claude <noreply@anthropic.com>
…e filter choices

The project-scoped algorithm list reverts to main's behavior — the algorithms
on the project's enabled pipelines — so a freshly configured project sees what
it can run before any job has completed. The used-only list (every algorithm
that produced results here, including superseded versions, detectors, and
standalone post-processing algorithms) moves to a new OccurrenceViewSet
sub-action at /occurrences/algorithms/, where it serves the occurrence
filter's choices so the filter never offers a zero-result value.

The enabled_in_project annotation and serializer field are dropped — the page
no longer mixes historical algorithms in, so there is nothing to gray out.

Also renames the occurrence queryset methods: detected_or_classified_by →
processed_by_algorithm (and the not_/_q variants), since the EXISTS matches
post-processing algorithms such as a size filter through the classification
leg — they neither detect nor classify. Future siblings can follow the same
family: processed_by_pipeline, processed_by_job.

Co-Authored-By: Claude <noreply@anthropic.com>
mihow and others added 2 commits July 22, 2026 16:47
…lgorithms/

The filter's choices now come from the used-only sub-action, so the picker
offers exactly the algorithms with results in the project. The algorithms
page keeps reading /ml/algorithms/, which lists the enabled-pipeline set.

Co-Authored-By: Claude <noreply@anthropic.com>
…tests

The scope-shape tests merged from main call _scoped_classifications with its
pre-idempotency two-argument signature; this branch added a third argument,
the masking algorithm whose lineage the scope excludes. A textual merge could
not catch the mismatch.

Co-Authored-By: Claude <noreply@anthropic.com>
@mihow
mihow force-pushed the fix/algo-filter-and-masking-idempotency branch from 85af4c0 to 6332ab5 Compare July 22, 2026 23:53
…zation path [skip ci]

Comment-only change. If a second post-processing task records applied_to
lineage and wants the same idempotency, the exclude should become a
ClassificationQuerySet method instead of being copied.

Co-Authored-By: Claude <noreply@anthropic.com>
@mihow
mihow merged commit 7832a1c into main Jul 23, 2026
5 checks passed
@mihow
mihow deleted the fix/algo-filter-and-masking-idempotency branch July 23, 2026 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants